-
Notifications
You must be signed in to change notification settings - Fork 368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Superoperator noise for density matrix method #295
Superoperator noise for density matrix method #295
Conversation
63b7002
to
212506a
Compare
@atilag I rebased to remove the src directory restructuring from this PR so its easier to review, I will add that as a final PR after the others are merged |
212506a
to
b59e13b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job again!
//----------------------------------------------------------------------- | ||
|
||
Superoperator() : Superoperator(0) {}; | ||
explicit Superoperator(size_t num_qubits); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What implicit conversion are we trying to avoid here? ... not sure if explicit makes sense.. but in any case, is not bothering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can assing num_qubits_
in the initializer list of the ctor:
explicit Superoperator(size_t num_qubits) : num_qubits_(set_num_qubits(num_qubits)){
}
this used to save some time at object construction time (at least the last time I saw it, It may not be the case now)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't compile for me
// Constructors & Destructor | ||
//------------------------------------------------------------------------------ | ||
|
||
template <typename data_t> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comment above (in the ctor)
} | ||
|
||
// Return the set of qobj snapshot types supported by the State | ||
virtual stringset_t allowed_snapshots() const override { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our TODO list, we have to decople Qobj as much as possible.. just in case we change the format in the future (Protobuffer... is coming...)
const std::vector<Operations::Op> &ops) { | ||
// An n-qubit unitary as 2^4n complex doubles | ||
// where each complex double is 16 bytes | ||
(void)ops; // avoid unused variable compiler warning |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove this ungly c-casting style by setting a default value for the ops
parameter...
..., onst std::vector<Operations::Op> &ops = {}) {
Did you have the chance to check if this generates another unsed variable warning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if it has a default value, it will still be unused in the function body
// An n-qubit unitary as 2^4n complex doubles | ||
// where each complex double is 16 bytes | ||
(void)ops; // avoid unused variable compiler warning | ||
size_t shift_mb = std::max<int_t>(0, num_qubits + 4 - 20); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why - 20
?
Can we add a comment or better, use a const
with a name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shift by -20 converts bytes to MiB
* Was using the wrong qubit number to calculate qubitvector multiplication indexes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merging!
* Add superoperator simulator * Add superoperator sampling method to noise model * Add superoperator noise to density matrix qasm controller * Remove AbstractError and add superop to QuantumError * Fix measure sampling test for density matrix method * Fix bug in superoperator matrix multiplication - Was using the wrong qubit number to calculate qubitvector multiplication indexes. * Use QasmBackendConfiguration
Summary
Adds support for general superoperator noise for the density matrix simulation method.
Depends on #253
TODO
Details and comments
Superoperator
class (subclass of DensityMatrix/QubitVector).QuantumError
class to convert toSuperoperator
via superoperator simulation of noise sub circuitsQasmSimulator
for the density matrix methodsrc/state-methods
folder, and only have controllers insrc/simulators
Example
An example of where this is useful is running noisy small qubit circuits with Kraus errors (eg something like randomized benchmarking simulations).
For the following example timing results on my laptop are: